Skip to content

fix: guard against nil ContextEvaluator func in memprovider Resolve#522

Open
somaz94 wants to merge 2 commits into
open-feature:mainfrom
somaz94:fix/memprovider-nil-context-evaluator
Open

fix: guard against nil ContextEvaluator func in memprovider Resolve#522
somaz94 wants to merge 2 commits into
open-feature:mainfrom
somaz94:fix/memprovider-nil-context-evaluator

Conversation

@somaz94

@somaz94 somaz94 commented Jul 7, 2026

Copy link
Copy Markdown

memprovider.InMemoryFlag.ContextEvaluator is typed *func(...). Resolve
only checked the pointer for nil, so a non-nil pointer to a nil func passed the
guard and panicked on the dereference-and-call. This adds the missing inner
nil-func check so such a flag falls back to the default variant, plus a
regression test.

Fixes #521

Validation:

  • make test (go test -tags testtools -race): all packages pass, memprovider at 86.9% coverage
  • make lint (golangci-lint v2.9.0): 0 issues
  • Confirmed the new test panics against the old guard and passes with the fix

@somaz94
fix: guard against nil ContextEvaluator func in memprovider Resolve
d7ee7b6 
Signed-off-by: somaz <genius5711@gmail.com>
@coderabbitai

coderabbitai Bot commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 119f42db-67b2-4a9f-955b-387b12c84107

📥 Commits

Reviewing files that changed from the base of the PR and between d7ee7b6 and f9fcb72.

📒 Files selected for processing (1)
  • openfeature/memprovider/in_memory_provider_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • openfeature/memprovider/in_memory_provider_test.go
📝 Walkthrough

Walkthrough

Modified InMemoryFlag.Resolve to guard both the ContextEvaluator pointer and underlying function before invocation. Added a regression test confirming fallback to DefaultVariant when the pointer references a nil function.

Changes

ContextEvaluator nil guard

Layer / File(s) Summary
Nil-function guard and regression test
openfeature/memprovider/in_memory_provider.go, openfeature/memprovider/in_memory_provider_test.go		 Resolve checks both the ContextEvaluator pointer and referenced function before calling it; the test verifies fallback to DefaultVariant with StaticReason without panicking.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the fix for nil ContextEvaluator handling in memprovider Resolve.
Description check ✅ Passed The description matches the code change and regression test, so it is clearly relevant.
Linked Issues check ✅ Passed The PR satisfies #521 by checking the underlying func for nil and verifying fallback behavior with a test.
Out of Scope Changes check ✅ Passed The changes stay focused on the nil ContextEvaluator fix and its regression test, with no unrelated additions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@somaz94
somaz94 marked this pull request as ready for review July 9, 2026 02:53
@somaz94
somaz94 requested review from a team as code owners July 9, 2026 02:53
@codecov

codecov Bot commented Jul 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.55%. Comparing base (e75753b) to head (f9fcb72).

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #522      +/-   ##
==========================================
+ Coverage   86.45%   86.55%   +0.09%     
==========================================
  Files          22       22              
  Lines        2119     2119              
==========================================
+ Hits         1832     1834       +2     
+ Misses        242      241       -1     
+ Partials       45       44       -1
Flag Coverage Δ
e2e 86.55% <100.00%> (+0.09%) ⬆️
unit 86.55% <100.00%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

erka
erka approved these changes Jul 19, 2026
View reviewed changes

@erka erka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @somaz94

One small nit: I don't think a subtest is necessary if there's only one test case.

Comment thread openfeature/memprovider/in_memory_provider_test.go Outdated
@somaz94
test: flatten single-case nil evaluator subtest per review
f9fcb72 
Signed-off-by: somaz <genius5711@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erka erka erka approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] memprovider.ContextEvaluator nil-check is insufficient — non-nil pointer to nil function panics

2 participants

@somaz94 @erka